Disallow function pointers to #[rustc_args_required_const] #48078

alexcrichton · 2018-02-08T21:13:14Z

This commit disallows acquiring a function pointer to functions tagged as
#[rustc_args_required_const]. This is intended to be used as future-proofing
for the stdsimd crate to avoid taking a function pointer to any intrinsic which
has a hard requirement that one of the arguments is a constant value.

Note that the first commit here isn't related specifically to this feature, but was necessary to get this working in stdsimd!

rust-highfive · 2018-02-08T21:13:24Z

r? @michaelwoerister

(rust_highfive has picked a reviewer for you, use r? to override)

alexcrichton · 2018-02-08T21:13:54Z

r? @eddyb

Thanks again for your excellent instructions here! To confirm we at least don't need this for stdsimd today in terms of methods, just free functions for intrinsics. (methods may come at a later date maybe though?)

kennytm · 2018-02-08T21:21:18Z

src/librustc_typeck/check/mod.rs

+                                      span: Span) {
+        // We're only interested in functions tagged with
+        // #[rustc_args_required_const], so ignore anything that's not.
+        if !self.tcx.get_attrs(def_id).iter().any(|a| a.check_name("rustc_args_required_const")) {


This could be !self.tcx.has_attr(def_id, "rustc_args_required_const")

Unfortunately left out it means that when the `#![feature(proc_macro)]` flag is in effect it fails to find `rustc_args_required_const` for expansion. This version, however, is verified to work with stdsimd's requirements!

eddyb · 2018-02-08T21:49:02Z

@alexcrichton Oh, what I was thinking wrt methods is irrelevant - you can't "extract" the method as a value any other way than using a value path - so your check here would work fine even then.

eddyb · 2018-02-08T22:11:06Z

src/librustc_typeck/check/mod.rs

+        }
+
+        self.tcx.sess.span_err(span, "this function can only be invoked \
+                                      directly, not through a function pointer");


~~Can you generalize this a bit?~~

Some context: foo(0) works but let f = foo; f(0) doesn't, even if it doesn't become a function pointer (if we allow the latter, we can't prevent let f = foo; f(0); f(1) easily/at all, which is the scary hazard, typesystem-wise).

~~So, "this function can only be called directly, not used as a value or called through a function pointer".~~

half an hour later okay maybe we can just keep your version, if it's confusing for anyone we can change it later.

eddyb · 2018-02-08T22:29:08Z

@alexcrichton Okay, I think the only change I want on this is a sanity check in case we screw up:

rust/src/librustc_trans/mir/rvalue.rs

Lines 195 to 197 in 932c736

    
           mir::CastKind::ReifyFnPointer => { 
        
               match operand.layout.ty.sty { 
        
                   ty::TyFnDef(def_id, substs) => {

rust/src/librustc_trans/mir/constant.rs

Lines 714 to 716 in 932c736

    
           mir::CastKind::ReifyFnPointer => { 
        
               match operand.ty.sty { 
        
                   ty::TyFnDef(def_id, substs) => {

rust/src/librustc_mir/interpret/eval_context.rs

Lines 716 to 718 in 932c736

    
           ReifyFnPointer => { 
        
               match self.eval_operand(operand)?.ty.sty { 
        
                   ty::TyFnDef(def_id, substs) => {

Those 3 places implement the reification to a fn pointer - can you add a span_bug! for the case when the attribute is present on def_id? Just so we ICE instead of doing the wrong thing.

alexcrichton · 2018-02-08T22:42:01Z

@bors: r=eddyb

Thanks again so much for the detailed help @eddyb!

bors · 2018-02-08T22:42:02Z

📌 Commit 5de8e19 has been approved by eddyb

This commit disallows acquiring a function pointer to functions tagged as `#[rustc_args_required_const]`. This is intended to be used as future-proofing for the stdsimd crate to avoid taking a function pointer to any intrinsic which has a hard requirement that one of the arguments is a constant value.

alexcrichton · 2018-02-08T22:46:41Z

@bors: r=eddyb

bors · 2018-02-08T22:46:42Z

📌 Commit 7a20fc1 has been approved by eddyb

…-proc-macro, r=eddyb Disallow function pointers to #[rustc_args_required_const] This commit disallows acquiring a function pointer to functions tagged as `#[rustc_args_required_const]`. This is intended to be used as future-proofing for the stdsimd crate to avoid taking a function pointer to any intrinsic which has a hard requirement that one of the arguments is a constant value. Note that the first commit here isn't related specifically to this feature, but was necessary to get this working in stdsimd!

Rollup of 12 pull requests - Successful merges: #47835, #47854, #48015, #48047, #48051, #48058, #48064, #47790, #48059, #48078, #48080, #48085 - Failed merges:

alexcrichton · 2018-02-09T18:37:49Z

@bors: rollup

…-proc-macro, r=eddyb Disallow function pointers to #[rustc_args_required_const] This commit disallows acquiring a function pointer to functions tagged as `#[rustc_args_required_const]`. This is intended to be used as future-proofing for the stdsimd crate to avoid taking a function pointer to any intrinsic which has a hard requirement that one of the arguments is a constant value. Note that the first commit here isn't related specifically to this feature, but was necessary to get this working in stdsimd!

Rollup of 17 pull requests - Successful merges: #47790, #47835, #47854, #48015, #48047, #48051, #48058, #48059, #48064, #48078, #48080, #48085, #48086, #48090, #48093, #48098, #48101 - Failed merges:

…-proc-macro, r=eddyb Disallow function pointers to #[rustc_args_required_const] This commit disallows acquiring a function pointer to functions tagged as `#[rustc_args_required_const]`. This is intended to be used as future-proofing for the stdsimd crate to avoid taking a function pointer to any intrinsic which has a hard requirement that one of the arguments is a constant value. Note that the first commit here isn't related specifically to this feature, but was necessary to get this working in stdsimd!

Rollup of 16 pull requests - Successful merges: #47790, #47835, #47854, #48015, #48047, #48051, #48058, #48059, #48064, #48078, #48080, #48086, #48098, #48101, #48107, #48100 - Failed merges:

Rollup of 20 pull requests - Successful merges: #47790, #47835, #47854, #48015, #48047, #48051, #48058, #48059, #48064, #48078, #48080, #48086, #48098, #48101, #48107, #48100, #48085, #48120, #48124, #47547 - Failed merges:

rust-highfive assigned michaelwoerister Feb 8, 2018

rust-highfive assigned eddyb and unassigned michaelwoerister Feb 8, 2018

alexcrichton mentioned this pull request Feb 8, 2018

Stable SIMD in Rust rust-lang/rfcs#2325

Merged

kennytm reviewed Feb 8, 2018

View reviewed changes

Add rustc_args_required_const to the feature whitelist

35dca7e

Unfortunately left out it means that when the `#![feature(proc_macro)]` flag is in effect it fails to find `rustc_args_required_const` for expansion. This version, however, is verified to work with stdsimd's requirements!

alexcrichton force-pushed the fix-required-const-and-proc-macro branch from ba31f4f to b5475af Compare February 8, 2018 21:39

arazabishov added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 8, 2018

eddyb reviewed Feb 8, 2018

View reviewed changes

alexcrichton force-pushed the fix-required-const-and-proc-macro branch from b5475af to 5de8e19 Compare February 8, 2018 22:41

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 8, 2018

alexcrichton force-pushed the fix-required-const-and-proc-macro branch from 5de8e19 to 7a20fc1 Compare February 8, 2018 22:46

kennytm mentioned this pull request Feb 9, 2018

Rollup of 12 pull requests #48077

Closed

bors added a commit that referenced this pull request Feb 9, 2018

Auto merge of #48077 - kennytm:rollup, r=kennytm

68df9d6

Rollup of 12 pull requests - Successful merges: #47835, #47854, #48015, #48047, #48051, #48058, #48064, #47790, #48059, #48078, #48080, #48085 - Failed merges:

kennytm mentioned this pull request Feb 9, 2018

Rollup of 17 pull requests #48104

Closed

kennytm mentioned this pull request Feb 10, 2018

Rollup of 20 pull requests #48113

Merged

alexcrichton mentioned this pull request Feb 10, 2018

Add #[rustc_args_required_const] annotations rust-lang/stdarch#319

Merged

bors merged commit 7a20fc1 into rust-lang:master Feb 10, 2018

alexcrichton deleted the fix-required-const-and-proc-macro branch February 26, 2018 23:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disallow function pointers to #[rustc_args_required_const] #48078

Disallow function pointers to #[rustc_args_required_const] #48078

alexcrichton commented Feb 8, 2018

rust-highfive commented Feb 8, 2018

alexcrichton commented Feb 8, 2018

kennytm Feb 8, 2018

alexcrichton Feb 8, 2018

eddyb commented Feb 8, 2018

eddyb Feb 8, 2018 •

edited

Loading

eddyb commented Feb 8, 2018

alexcrichton commented Feb 8, 2018

bors commented Feb 8, 2018

alexcrichton commented Feb 8, 2018

bors commented Feb 8, 2018

alexcrichton commented Feb 9, 2018

Disallow function pointers to #[rustc_args_required_const] #48078

Disallow function pointers to #[rustc_args_required_const] #48078

Conversation

alexcrichton commented Feb 8, 2018

rust-highfive commented Feb 8, 2018

alexcrichton commented Feb 8, 2018

kennytm Feb 8, 2018

Choose a reason for hiding this comment

alexcrichton Feb 8, 2018

Choose a reason for hiding this comment

eddyb commented Feb 8, 2018

eddyb Feb 8, 2018 • edited Loading

Choose a reason for hiding this comment

eddyb commented Feb 8, 2018

alexcrichton commented Feb 8, 2018

bors commented Feb 8, 2018

alexcrichton commented Feb 8, 2018

bors commented Feb 8, 2018

alexcrichton commented Feb 9, 2018

eddyb Feb 8, 2018 •

edited

Loading